fix: wrap setup.sh escape hatch with 300s timeout to prevent indefinite build hang - #23

Research Review

Verified

• Root cause location: scripts/docker-entrypoint.sh:261-264 on current main (freshly cloned/pulled to /usercontent/python-runner) matches exactly what the PR body and issue My App Python: setup.sh escape hatch has no timeout, blocking build forever #22 cite — if [ -f "setup.sh" ]; then ... ./setup.sh; fi with no timeout.
• "No loading-server mechanism" claim: independently confirmed. grep -rniE "loading-server|LOADING_PID|healthz|error-page|supervisor" scripts/ and a repo-wide search for any HTTP server file/http.createServer/.listen( returns nothing in this repo. This is a real, verified divergence from php-runner, not an assumption.
• set -e propagation claim: confirmed set -e is set at scripts/docker-entrypoint.sh:2, and the existing pip install steps immediately above the setup.sh block have no || true/guard, so a failure there already terminates the script via set -e. The PR's framework — "both branches just exit with the failure code, which is functionally identical to existing failure handling" — is accurate for this repo, not invented.
• timeout binary availability: Dockerfile base image is python:3.12-slim (Debian), and timeout ships in coreutils, an essential Debian package always present — not something that needs a new install step. No gap here.
• Sibling consistency (php-runner#9): pulled the actual php-runner PR diff. Both PRs use timeout 300s, both treat exit 124 and any other non-zero exit as terminal failure. The only difference is the terminal-failure mechanism (php-runner kills its loading server and execs an error page; python-runner just exits the script) — and that divergence is the one explicitly verified and documented in the PR body/Lessons, not an unexplained departure from the sibling pattern.
• Self-reported test results: reproduced independently on the PR branch (pr-23, head da25430). bash -n scripts/docker-entrypoint.sh — syntax OK. New tests/test-entrypoint-setup-timeout.sh — 7/7 passed, matching the claimed count exactly (including the two live behavioral tests: hanging setup.sh killed at ~1s elapsed with a 1s test timeout resolving to exit 124; fast-fail setup.sh reports exit 3, not conflated with timeout). All 4 pre-existing suites also reproduced clean: credential-header-auth 13/13, credential-scrub 6/6, fragment-branch 8/8, subpath 7/7 — no regression.
• Attribution/process: PR body contains both Co-Authored-By: Claude and 🤖 Generated with [Claude Code], and a filled-in ## Lessons section (not a placeholder) that explains the real, verified finding (no loading-server in this repo) rather than a generic statement.

Adjacent Stubs Detected

• None found. Swept the full scripts/docker-entrypoint.sh (411 lines) for TODO/FIXME/XXX/HACK, undefined : undefined, || undefined, bare console.log/empty catch, and stray return 0/return null patterns. The only return 0 occurrences are legitimate early-returns inside helper functions (has_package, find_app_module, git-metadata gathering) unrelated to this change.

Orphaned Files Detected

• None. The new tests/test-entrypoint-setup-timeout.sh is a manually-invoked shell test script (script entry point pattern), not an imported module — the orphan-file check doesn't apply to it.

Escape Hatch Coverage

• scripts/docker-entrypoint.sh:264-271 (new): this PR adds the first terminal-rejection branch on setup.sh — a setup.sh that legitimately needs more than 300s (e.g., large model download, slow native compile) will now be hard-killed and the build permanently failed, where previously it would eventually succeed no matter how long it took. No bypass/override is claimed in the PR body, so there's no unbacked-claim gap to flag. A passing-path test exists (Test 7: well-behaved setup.sh still succeeds under the wrapper) — this satisfies the "at least one success-path test" requirement, though it only proves a fast setup.sh still passes, not that a legitimately slow one has any way through. Not blocking: the 300s bound and lack of a configurable override is the same tradeoff already shipped and presumably accepted in the merged php-runner sibling (php-runner#9), so this isn't a novel gap introduced by this PR — it's inherited, matching-the-family behavior. Worth flagging for the family as a whole, not this PR specifically.

Object Shape Verification

• N/A — no spreads, merges, or re-serialization of fetched/queried objects in this diff (pure shell script + shell test file).

Contract diff

• No Fastify/Next.js/MCP request handler is changed by this diff — docker-entrypoint.sh runs at container boot/build time, not on a live request path, so the request-handler contract-diff protocol doesn't directly apply. Treating the boot script's own "contract" loosely: previously a slow-but-eventually-successful setup.sh always succeeded (no matter how long); after this PR, anything over 300s now fails permanently. That is a genuine behavior change, but it is exactly the fix requested by issue My App Python: setup.sh escape hatch has no timeout, blocking build forever #22 ("blocking build forever" → bounded and terminal), consistent with the same change already accepted for php-runner. The 300s value has no cited measurement (no Loki/Grafana query or benchmark of real-world setup.sh durations in the PR body), but this is a build-time script, not a request path behind the 60s nginx ingress ceiling, so the "no measurement on a request-path timeout blocks merge" rule doesn't strictly apply here — flagging as worth verifying rather than blocking, consistent with how the merged php-runner sibling was treated. Files checked for other handler/timeout changes: scripts/docker-entrypoint.sh (only change), tests/test-entrypoint-setup-timeout.sh (new test file, no handler).

Impact Analysis

• Blast radius: affects the Docker entrypoint used by every My App built with the Python runner. Per the linked issue and PR body, the fix only affects builds going forward (fork sync + service remake needed for existing deployed instances) — consistent with the "manual follow-up, not a merge blocker" pattern already established in the sibling PRs.
• Downstream consumers: none within this repo beyond the entrypoint itself; deploy-manager triggers builds via this runner's image but does not read this script directly.
• Coordinated changes needed: fork sync (eyevinn-osaas/eyevinn-osaas-dev) + service remake to take effect on already-deployed apps — a manual op for birme after merge, same as noted in the php-runner sibling PR. No PR-blocking action needed.
• Breaking change risk: low-to-medium — the only behavior change is that a setup.sh running longer than 300s now fails the build instead of eventually succeeding. This is the intended fix, not an unintended side effect, and matches the accepted sibling precedent.

Risks Not Tested

• No CI workflow exists in this repo at all (.github/ is absent) — every test file here, old and new, is developer-run only. The "7/7 passed" and pre-existing-suite counts were verified live in this review, but there is no automated gate preventing a future regression to this file from going unnoticed. This is pre-existing repo state, not introduced by this PR, so not a blocker for this change, but worth a follow-up ticket if the team wants CI parity with php-runner (which does have a partial test.yml, per that sibling PR's own body).
• Real-world setup.sh duration distribution across existing My Apps is not measured anywhere in this PR or the linked issues — the 300s bound is inherited from the php-runner sibling's choice, not independently benchmarked against actual python-runner usage.
• Live behavioral confirmation (in an actual deployed My App triggering a real build hang) was not performed — only the extracted-logic sandbox tests were run, both by the PR author and by me in this review.

Research Suggestions

• Consider filing a follow-up ticket to add a .github/workflows/test.yml to python-runner mirroring php-runner's structure, so tests/*.sh (including this new one) run in CI rather than only locally.
• If any real My App's setup.sh is known to legitimately exceed 300s (e.g., large ML dependency builds), that should surface as a fast-follow ticket for a configurable timeout — same open question applies to the merged php-runner sibling.

No blocking issues found. Research is well-grounded: the PR's central claim (this repo has no loading-server mechanism, unlike php-runner) was independently verified against the actual source rather than assumed, line numbers match current main, and all self-reported test results reproduce exactly.

Reviewed by PR Research Validator. This review checks whether code is based on verified sources rather than assumptions.